Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Add e2e test for abort in renderer loaded native module #617

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Jan 9, 2023

Uses sadness-generator native module to test abort.

Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great package name

@timfish
Copy link
Collaborator Author

timfish commented Jan 9, 2023

great package name

Oh, it's completely stolen from the Rust crate that it wraps.

I think we can thank @Jake-Shadle for this 🤣

@timfish
Copy link
Collaborator Author

timfish commented Jan 9, 2023

The Windows test failures show that Electron notifies us of the renderer crash but no minidump was found:

[App] [    Main] Sentry Logger [log]: 'renderer' process 'crashed'
[App] [    Main] Renderer process crashed - see https://www.electronjs.org/docs/tutorial/application-debugging for potential debugging information.
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[Test Context] Timeout: Waiting 15000ms for 1 events. Only 0 events received

@timfish
Copy link
Collaborator Author

timfish commented Jan 9, 2023

I added some crude retrying and it appears there just isn't a minidump generated for abort on Windows:

[App] [    Main] Sentry Logger [log]: 'renderer' process 'crashed'
[App] [    Main] Renderer process crashed - see https://www.electronjs.org/docs/tutorial/application-debugging for potential debugging information.
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[App] [    Main] Sentry Logger [log]: Found 0 minidumps
[Test Context] Timeout: Waiting 15000ms for 1 events. Only 0 events received

@AbhiPrasad
Copy link
Member

I added some crude retrying and it appears there just isn't a minidump generated for abort on Windows:

huh - is that something that is just a limitation for electron?

@timfish
Copy link
Collaborator Author

timfish commented Jan 9, 2023

is that something that is just a limitation for electron?

The docs for sadness_generator::raise_abort say:

Note that on Windows, std::process::abort, the canonical way to abort processes in Rust, uses the fastfail intrinsic, which neither raises a SIGABRT signal, nor issue a Windows exception. The method in this library always uses libc::abort

So it could be that Crashpad on Windows doesn't cater for whatever Rust + libc::abort does 🤷‍♂️.

Just testing with the upstream Electron reporter now...

@timfish
Copy link
Collaborator Author

timfish commented Jan 9, 2023

Yes, confirmed that the Electron crashReporter does not send a minidump on Windows due to sadness_generator::raise_abort().

I want to double check C abort() to ensure it's not something specific to a Rust native module....

@timfish
Copy link
Collaborator Author

timfish commented Jan 9, 2023

With the following basic Node-API module I get minidumps on macOS and Linux but nothing on Windows. That suggest this is an upstream issue with Electron.

#include <node_api.h>
#include <stdlib.h>

namespace demo
{
  napi_value Method(napi_env env, napi_callback_info args)
  {
    abort();
    return nullptr;
  }

  napi_value init(napi_env env, napi_value exports)
  {
    napi_status status;
    napi_value fn;
    status = napi_create_function(env, nullptr, 0, Method, nullptr, &fn);
    if (status != napi_ok)
      return nullptr;
    status = napi_set_named_property(env, exports, "abort", fn);
    if (status != napi_ok)
      return nullptr;
    return exports;
  }

  NAPI_MODULE(NODE_GYP_MODULE_NAME, init)
}

@timfish
Copy link
Collaborator Author

timfish commented Mar 18, 2024

Just merged from master to see if this has been fixed in any of the recent versions of Electron...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants